Add sandbox capabilities matrix benchmark and weekly workflow - #321
Add sandbox capabilities matrix benchmark and weekly workflow#321kisernl wants to merge 11 commits into
Conversation
Co-Authored-By: Noah Kiser <noah@computesdk.com>
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
Contributor License AgreementAll contributors are covered by a CLA. |
| opts: { resultsDir: string; runConfig: ResolvedRunConfig }, | ||
| ): Promise<void> { | ||
| const results: CapabilityMatrixResult[] = participants.map((participant) => { | ||
| const record = participant.records[0]; |
There was a problem hiding this comment.
🟡 Capability results can report a random run instead of the first one
The capability matrix is taken from whichever run finished first (participant.records[0] at benchmarks/sandbox/capabilities-results.ts:38) rather than the first run that was started, so when more than one run per provider is requested the published matrix can come from an arbitrary run.
Impact: With more than one iteration, the saved feature matrix may reflect an arbitrary run rather than a deterministic one, making results non-reproducible.
Records arrive in completion order, so index 0 is not iteration 0
benchmarks/src/util/records.ts documents that "Platform records arrive in completion order, while the legacy result files list iterations in launch order, so every legacy bridge orders records by taskIndex" — every other writer (e.g. benchmarks/sandbox/legacy-results.ts:54) calls byTaskIndex(participant.records) first. The new writer indexes participant.records[0] directly. The config default is iterations: 1 (benchmarks/sandbox/capabilities.bench.ts:69), but the CLI can override --iterations/--concurrency, in which case records[0] is whichever task finished first and the remaining matrices are silently dropped.
Prompt for agents
In benchmarks/sandbox/capabilities-results.ts the aggregation picks participant.records[0], but platform records arrive in completion order (see the comment in benchmarks/src/util/records.ts). Other result writers normalize with byTaskIndex() before mapping. Consider ordering records by taskIndex before taking the first one (and decide explicitly how multiple iterations should be combined, e.g. first iteration or OR/AND across iterations).
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Good catch — fixed. The writer now:
- Orders
participant.recordswithbyTaskIndex()before mapping them. - Aggregates all per-iteration feature matrices with an AND rule: a feature only passes if every iteration passed, and the error comes from the first failing iteration.
This makes the published matrix deterministic for any --iterations override while still defaulting to the single-run case.
Co-Authored-By: Noah Kiser <noah@computesdk.com>
Co-Authored-By: Noah Kiser <noah@computesdk.com>
| if: env.SHOULD_RUN == 'true' | ||
| run: | | ||
| if [ "${{ github.event_name }}" = 'schedule' ]; then | ||
| pnpm update |
There was a problem hiding this comment.
P1: Scheduled benchmark runs use pnpm update instead of frozen lockfile install
Scheduled runs automatically update dependencies via pnpm update before loading vault secrets
Use pnpm install --frozen-lockfile in all CI jobs; update deps in a separate PR workflow
AI prompt
Check if this security scanner issue is valid. If so, understand the root cause and fix it. If appropriate, update or add tests. Keep the change focused and preserve intended behavior.
<file name=".github/workflows/sandbox-capabilities.yml">
<violation number="1" location=".github/workflows/sandbox-capabilities.yml:92">
<priority>P1</priority>
<title>Scheduled benchmark runs use pnpm update instead of frozen lockfile install</title>
<evidence>The bench job's 'Install dependencies' step runs `pnpm update` when `github.event_name == 'schedule'`, automatically updating all dependencies to the latest semver-compatible versions without human review. Immediately after, the workflow loads all vault secrets and executes benchmark code via `npx tsx`, creating a supply-chain attack surface: a compromised dependency published between weekly runs would be automatically installed and executed with access to all provider credentials.</evidence>
<recommendation>Use `pnpm install --frozen-lockfile` unconditionally in both the `bench` and `collect` jobs. If periodic dependency updates are required, run them in a separate dedicated workflow that does not access vault secrets and does not execute benchmark code.</recommendation>
</violation>
</file>
| - name: Run capabilities benchmark | ||
| if: env.SHOULD_RUN == 'true' | ||
| run: | | ||
| . benchmarks/scripts/load-vault-secrets.sh '.*' |
There was a problem hiding this comment.
P2: Matrix jobs load all vault secrets via wildcard instead of provider-scoped access
Each matrix job loads all vault secrets with a wildcard regex
Pass only the current provider's name/matrix variable to the vault script
AI prompt
Check if this security scanner issue is valid. If so, understand the root cause and fix it. If appropriate, update or add tests. Keep the change focused and preserve intended behavior.
<file name=".github/workflows/sandbox-capabilities.yml">
<violation number="1" location=".github/workflows/sandbox-capabilities.yml:102">
<priority>P2</priority>
<title>Matrix jobs load all vault secrets via wildcard instead of provider-scoped access</title>
<evidence>Every matrix job sources `benchmarks/scripts/load-vault-secrets.sh '.*'`, loading every secret from the vault. Since the workflow matrix spans 29 different providers, the job testing a single provider receives credentials for all other providers as well, violating least-privilege and increasing the blast radius of any provider-specific breach.</evidence>
<recommendation>Scope secret loading to only the current matrix provider by passing `${{ matrix.provider }}` or a provider-specific vault path prefix to the load script instead of the `'.*'` wildcard.</recommendation>
</violation>
</file>
Co-Authored-By: Noah Kiser <noah@computesdk.com>
…s, avoid empty KEYS regex Co-Authored-By: Noah Kiser <noah@computesdk.com>
Co-Authored-By: Noah Kiser <noah@computesdk.com>
Co-Authored-By: Noah Kiser <noah@computesdk.com>
Co-Authored-By: Noah Kiser <noah@computesdk.com>
Co-Authored-By: Noah Kiser <noah@computesdk.com>
Co-Authored-By: Noah Kiser <noah@computesdk.com>
Co-Authored-By: Noah Kiser <noah@computesdk.com>
Summary
Adds a new
sandbox-capabilitiesbenchmark that creates one sandbox per provider and probes the full ComputeSDK surface (sandbox lifecycle, commands, filesystem, snapshots, templates, streaming, backgrounding, and port exposure). Each capability is a namedctx.stepwith its own try/catch so a single unsupported feature only fails that feature and does not abort the rest.Results are aggregated per-provider into a feature matrix (
{ passed, error? }per feature) and written toresults/sandbox-capabilities/<YYYY-MM-DD>.jsonpluslatest.json.Also adds the GitHub Actions workflow:
workflow_dispatchwith optional provider filter,iterations,concurrency, and apublishboolean.publishis false the results are still rendered as a GitHub Actions step summary (provider × feature pass/fail table with failure details), but they are not committed back to the repo.merge-results.tsnow knows how to combinesandbox-capabilitiesartifacts from the per-provider matrix jobs into a singlelatest.json.Link to Devin session: https://app.devin.ai/sessions/f98952f689154db886036dda3dd56bd1
Requested by: @kisernl